Codecov Report

Merging #2472 (010b7cd) into main (07b19ad) will decrease coverage by 0.01%.
The diff coverage is n/a.

❗ Current head 010b7cd differs from pull request most recent head dea083e. Consider uploading reports for the commit dea083e to get more accurate results

@@            Coverage Diff             @@
##             main    #2472      +/-   ##
==========================================
- Coverage   92.62%   92.60%   -0.02%     
==========================================
  Files         137      137              
  Lines        5017     5017              
  Branches     1060     1060              
==========================================
- Hits         4647     4646       -1     
- Misses        370      371       +1     

I think this should still remain es5, you don't have es2017 in browser

I agree with @obecny especially since the ESM build exists mostly to serve the browser usecase.

@obecny @dyladan I'm curious that what browsers that support ESM whilst don't have features like await support? I've copy-pasted the version matrix of browser features in the OP. It seems to me that native ESM support was landed quite late although it is part of es6.

@obecny @dyladan I'm curious that what browsers that support ESM whilst don't have features like await support? I've copy-pasted the version matrix of browser features in the OP. It seems to me that native ESM support was landed quite late although it is part of es6.

I doubt there are any browsers which support native ESM but don't support await, but I wasn't really thinking of native ESM support as much as build tooling. After thinking a little more though I think this is probably fine. The non-esm builds already target 2017 so I'm not sure why esm was pointing to es5 in the first place. ESM builds were contributed by @t2t2 in #2112 so perhaps they can shed some light on that?

@obecny @dyladan I'm curious that what browsers that support ESM whilst don't have features like await support? I've copy-pasted the version matrix of browser features in the OP. It seems to me that native ESM support was landed quite late although it is part of es6.

I doubt there are any browsers which support native ESM but don't support await, but I wasn't really thinking of native ESM support as much as build tooling. After thinking a little more though I think this is probably fine. The non-esm builds already target 2017 so I'm not sure why esm was pointing to es5 in the first place. ESM builds were contributed by @t2t2 in #2112 so perhaps they can shed some light on that?

hmm you right let's wait for @t2t2 would like to know if there were reasons for that or it was just mistake.

@t2t2 isn't a regular contributor so i'm not sure how quickly he will reply here. We can wait for some time, but if it doesn't come soon i'd rather not wait too long.

@t2t2 isn't a regular contributor so i'm not sure how quickly he will reply here. We can wait for some time, but if it doesn't come soon i'd rather not wait too long.

we can wait 24 hours and then just move forward

Yeah, while browsers that support esm modules also support modern syntax like async/await, the issue is you generally don't see much usage of esm modules directly via browser as no matter how much http2 coolaid you've consumed, it's performance has still been awful for real apps (webpack blog post in 2016, vue/vite author in 2021). At most the use case for <script type="module"> has been as a feature switch w/ nomodule to provide modern v older bundles or in local development (but even then they've pre-bundled all files in a dependency together because even in localhost it's been awful for browsers to load all files - vite docs, snowpack docs).

So this leaves esm files to be mainly used by bundlers for tree shaking. Good news here is bundlers don't care about how modern syntax is. Bad news is bundlers don't care about how modern syntax is - it's the job of transpilers like babel. So for users who still want to support older browsers (IE) they'd need dependencies to work in them or transpile it themselves.

So let's check webpack's babel-loader documentation

Usage

Within your webpack configuration object, you'll need to add the babel-loader to the list of modules, like so:

module: {
  rules: [
    {
      test: /\.m?js$/,
      exclude: /(node_modules|bower_components)/,
      use: {
        loader: 'babel-loader',
        options: {
          presets: ['@babel/preset-env']
        }
      }
    }
  ]
}

[...]

Troubleshooting

babel-loader is slow!

Make sure you are transforming as few files as possible. Because you are probably matching /\.m?js$/, you might be transforming the node_modules folder or other unwanted source.

To exclude node_modules, see the exclude option in the loaders config as documented above.

How about rollup's babel plugin:

External dependencies

Ideally, you should only be transforming your source code, rather than running all of your external dependencies through Babel (to ignore external dependencies from being handled by this plugin you might use exclude: 'node_modules/**' option). If you have a dependency that exposes untranspiled ES6 source code that doesn't run in your target environment, then you may need to break this rule, but it often causes problems with unusual .babelrc files or mismatched versions of Babel.

We encourage library authors not to distribute code that uses untranspiled ES6 features (other than modules) for this reason. Consumers of your library should not have to transpile your ES6 code, any more than they should have to transpile your CoffeeScript, ClojureScript or TypeScript.

Use babelrc: false to prevent Babel from using local (i.e. to your external dependencies) .babelrc files, relying instead on the configuration you pass in.

So that's 2 most popular ways saying don't transpile your dependencies (= dependencies should ship older code). This has way of thinking challenged in default templates for frameworks, for example create-react-app switched to transpiling dependencies by default in v2, vue-cli offers a config option for dependencies to transpile, angular is the only one explicilty saying target es2015; and also from some library authors (preact's microbundlers modern mode, svelte author's opinion)

Question here becomes can you really afford to go modern considering who's gonna use opentelemetry. Like if I were to create/maintain a library that I'd expect users to be more up to date on, yeah I'd provide a modern build so those who don't use IE can enjoy smaller size (or if IE support was to be expected write code in a way that avoids newer features). But in case of opentelemetry you can expect a lot more users who want to throw it to some old angularjs app they have with a webpack config that is held together with copy-pasted snippets from ages ago, and that still want IE support. So for ESM pr the less breaking way was to just match cjs targets so bundlers that grabbed esm files without transpiling still worked.

So basically there's 2 options:

1. Just ship esm but older syntax code and it will just work for most users
2. Ship modern-er esm builds and include plenty of warnings and instructions on how to make your dev tooling transpile otel dependencies to match your target browsers (and same goes for everyone who distributes their own prepackaged versions via npm)

Consider me another vote for "just leave it at esm5".

Thank you @t2t2 for the explanation.

@legendecas I assume since you 👍 his suggestion to leave as es5 you are ok with closing this PR?